Skip to content

Delay dataset layout copy to DCPL #5537

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

mattjala
Copy link
Contributor

@mattjala mattjala commented May 15, 2025

  • Optimize dataset opening by delaying the copy of dataset layout to its internal DCPL

Copying the layout immediately slows down workloads that open many small datasets.

The layout copy is delayed until either a user requests the dataset's DCPL, or until the start of an operation that may need to read the layout from the DCPL.

Whether the copying of the layout has been performed yet for a dataset is tracked by a new field on H5D_shared_t.

  • Added a new test for this behavior in dsets.c

@mattjala mattjala self-assigned this May 15, 2025
@mattjala mattjala added the Priority - 2. Medium It would be nice to have this in the next release label May 15, 2025
@mattjala mattjala requested a review from brtnfld as a code owner May 15, 2025 19:57
@mattjala mattjala added the Component - C Library Core C library issues (usually in the src directory) label May 15, 2025
@github-project-automation github-project-automation bot moved this to To be triaged in HDF5 - TRIAGE & TRACK May 15, 2025
@nbagha1 nbagha1 added the HDFG-internal Internally coded for use by the HDF Group label May 16, 2025
@nbagha1 nbagha1 moved this from To be triaged to Scheduled/On-Deck in HDF5 - TRIAGE & TRACK May 16, 2025
@ajelenak ajelenak added this to the Release 2.0.0 milestone May 16, 2025
@nbagha1 nbagha1 moved this from Scheduled/On-Deck to In progress in HDF5 - TRIAGE & TRACK May 16, 2025
@@ -3021,6 +3033,10 @@ H5D__check_filters(H5D_t *dataset)
if (fill_status == H5D_FILL_VALUE_DEFAULT || fill_status == H5D_FILL_VALUE_USER_DEFINED) {
if (fill->fill_time == H5D_FILL_TIME_ALLOC ||
(fill->fill_time == H5D_FILL_TIME_IFSET && fill_status == H5D_FILL_VALUE_USER_DEFINED)) {
/* Flush layout to DCPL before reading */
if (H5D_flush_layout_to_dcpl(dataset) < 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does H5Z_can_apply require a valid layout? I wonder if we could pass the layout separately.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not now that I think about it, though maybe we could hack it to work for internal filters

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understand it, H5Z_can_apply() eventually invokes filter-defined callbacks to determine whether the filter can be applied with the current DCPL/datatype/dataspace setup. The callback can access arbitrary fields on the DCPL including the layout, so we want the layout available there to reflect the dataset accurately.

@qkoziol
Copy link
Collaborator

qkoziol commented May 20, 2025

I have significantly changed this piece of code (and related areas) in my incoming threadsafety patches. Would you consider backburnering this until those are in?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component - C Library Core C library issues (usually in the src directory) HDFG-internal Internally coded for use by the HDF Group Priority - 2. Medium It would be nice to have this in the next release
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

5 participants